Skip to content

createkubertetesbinariesiso: Saving images in network and dashboard yaml#4459

Merged
yadvr merged 1 commit into
apache:4.14from
shapeblue:fix-cks-bin
Nov 20, 2020
Merged

createkubertetesbinariesiso: Saving images in network and dashboard yaml#4459
yadvr merged 1 commit into
apache:4.14from
shapeblue:fix-cks-bin

Conversation

@davidjumani
Copy link
Copy Markdown
Contributor

@davidjumani davidjumani commented Nov 11, 2020 •
edited
Loading

Description

Downloads images int the network and dashboard yaml while creating the kubernetes binaries iso
This step was skipped, so bringing up a cluster on a node that does not have access to the internet failed since the weavenet and dashboard images were not found locally

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

./create-kubernetes-binaries-iso.sh ./ 1.11.4 0.7.1 1.11.1 https://github.com/weaveworks/weave/releases/download/latest_release/weave-daemonset-k8s-1.11.yaml https://raw.githubusercontent.com/kubernetes/dashboard/v1.10.0/src/deploy/recommended/kubernetes-dashboard.yaml
..........
Downloading docker image k8s.gcr.io/kube-apiserver-amd64:v1.11.4 ---
Downloading docker image k8s.gcr.io/kube-controller-manager-amd64:v1.11.4 ---
Downloading docker image k8s.gcr.io/kube-scheduler-amd64:v1.11.4 ---
Downloading docker image k8s.gcr.io/kube-proxy-amd64:v1.11.4 ---
Downloading docker image k8s.gcr.io/pause:3.1 ---
Downloading docker image k8s.gcr.io/etcd-amd64:3.2.18 ---
Downloading docker image k8s.gcr.io/coredns:1.1.3 ---
Downloading docker image weaveworks/weave-kube:2.7.0 ---                       << Added image from network yaml
Downloading docker image weaveworks/weave-npc:2.7.0 ---                        << Added image from network yaml
Downloading docker image k8s.gcr.io/kubernetes-dashboard-amd64:v1.10.0 ---     << Added image from dashboard yaml

@davidjumani davidjumani reopened this Nov 11, 2020
@shwstppr
Copy link
Copy Markdown
Contributor

shwstppr commented Nov 11, 2020

Nice! Thanks @davidjumani. Will test this out

@davidjumani davidjumani force-pushed the fix-cks-bin branch 4 times, most recently from 4860f60 to bf3c782 Compare November 11, 2020 09:11
@shwstppr
Copy link
Copy Markdown
Contributor

shwstppr commented Nov 18, 2020

@davidjumani should this go in 4.14? And do we need to make any change in docs (added a new param for script)?

DaanHoogland
DaanHoogland reviewed Nov 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question

Comment thread scripts/util/create-kubernetes-binaries-iso.sh
for i in ${network_conf_file} ${dashboard_conf_file}
do
images=`grep "image:" $i | cut -d ':' -f2- | tr -d ' ' | tr -d "'"`
output=`printf "%s\n" ${output} ${images}`
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printf has only one specifier in the format and two arguments. What is intended here?

Copy link
Copy Markdown
Contributor Author

@davidjumani davidjumani Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a simpler way of doing printf "%s\n%s\n" ${output} ${images} :D

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, never encountered that before, tnx

@davidjumani davidjumani changed the base branch from master to 4.14 November 18, 2020 10:54
@davidjumani
Copy link
Copy Markdown
Contributor Author

davidjumani commented Nov 18, 2020

@davidjumani should this go in 4.14? And do we need to make any change in docs (added a new param for script)?

@shwstppr Don't think it's needed as it's an optional parameter and used in automated build jobs

DaanHoogland
DaanHoogland reviewed Nov 18, 2020
View reviewed changes
Comment thread scripts/util/create-kubernetes-binaries-iso.sh
# specific language governing permissions and limitations
# under the License.

set -e
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🇹🇷 red flag ;) but fine by me. we allow this script to exit and leave garbage behind. in view of the function of this script fine. (a comment or so would be appropriate, i think)

DaanHoogland
DaanHoogland approved these changes Nov 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

shwstppr
shwstppr approved these changes Nov 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Nov 19, 2020

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Nov 19, 2020

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Nov 19, 2020

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2407

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Nov 19, 2020

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Nov 19, 2020

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Nov 19, 2020

Trillian test result (tid-3213)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30086 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4459-t3213-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@davidjumani davidjumani added this to the 4.14.1.0 milestone Nov 20, 2020
@yadvr yadvr merged commit 1398dff into apache:4.14 Nov 20, 2020
@yadvr yadvr mentioned this pull request Dec 31, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shwstppr shwstppr shwstppr approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@andrijapanicsb andrijapanicsb Awaiting requested review from andrijapanicsb

Assignees

No one assigned

Labels

component:kubernetes type:bug

Projects

None yet

Milestone

4.14.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@davidjumani @shwstppr @DaanHoogland @blueorangutan @yadvr @svenvogel